[SPARK-53355][PYTHON][SQL] test python udf type behavior #52105
Conversation
benrobby
changed the title
|
@HyukjinKwon and @asl could you take a look? |
|
|
||
| return [ | ||
| ("byte_values", ByteType(), df([(-128,), (127,), (0,)])), | ||
| ("byte_null", ByteType(), df([(None,), (42,)])), |
There was a problem hiding this comment.
should we add a comment with context about the null handling importance for input type vs. return type?
There was a problem hiding this comment.
Would you have an example, what comment are you looking for exactly?
There was a problem hiding this comment.
optional nit, but i meant we could clarify the context for the return_types and input_types
HyukjinKwon
changed the title
| @@ -0,0 +1,5 @@ | |||
| These tests capture input/output type interfaces between python udfs and the engine. | |||
There was a problem hiding this comment.
Do we want to run this in CI? If so, we should add __init__.py ,and add this modul into dev/sparktestsupport/modules.py
There was a problem hiding this comment.
yes, let me try to add it. Do we need any other reviewer for the infra changes?
|
My only concern is that I don't think we should say this is the standard behaviours ... as some of behaviours are weird. I am fine with this change as long as we're all on the same page that some behaviours here might change in the future. |
Yes, agreed. I've also updated the readme to reflect that these tests are purely internal and not an API documentation |
benrobby
changed the title
benrobby
changed the title
dev/sparktestsupport/modules.py
Outdated
| ], | ||
| ) | ||
|
|
||
| pyspark_types = Module( |
There was a problem hiding this comment.
I am kind of hesitant whether it is too much to add a new testing module.
maybe we can just add these tests in pyspark-sql?
There was a problem hiding this comment.
I don't have a strong preference. @HyukjinKwon ?
There was a problem hiding this comment.
pyspark-sql alone should be fine
There was a problem hiding this comment.
alright, I moved the test to run under pyspark-sql
benrobby
changed the title
|
@HyukjinKwon this one is ready from my side :) |
|
Merged to master. |
|
@benrobby the new test fails in two scheduled jobs: would you mind taking a look? If it is acceptable, then we can skip it in the two envs. |
Thanks for flagging. This is caused by older numpy versions implementing |
|
@zhengruifeng here's the fixup pr #52247 :) |
### What changes were proposed in this pull request? - this is a minor followup to #52105, we noticed that the test breaks in two spark master runs with a different env - the root cause was that numpy 1.x implements `__repr__` differently ### Why are the changes needed? - fix `Build / Python-only (master, Minimum dependencies of PySpark)` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? ran tests locally with numpy 1.22.4 ### Was this patch authored or co-authored using generative AI tooling? No Closes #52247 from benrobby/SPARK-53355-fix-numpy-repr. Authored-by: Ben Hurdelhey <ben.hurdelhey@databricks.com> Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
No
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
Yes, used claude to generate the table printing utils. Generated-by: claude-sonnet-4